How do you plan to represent image data with this? Base64?

I don't expect raw image data to appear in the string representation; it'd only be useful to API clients interacting with binary directly (a la screenshot-raw).

For Windows and Linux, an event monitoring thread will be used, and data need to be kept alive for Linux, so there needs to be init/uninit functions for the clipboard stuffs so that mpv can clean up the mess when quitting. How about making them take a struct clipboard_ctx parameter with the init and uninit functions? They don't need to do anything on macOS.

Also on Linux, since this clipboard API is headless and has no VO dependence, it's possible that both X11 and Wayland servers are running, so it requires two separate monitoring threads to handle requests from both window systems. I'm not sure if this means that the API needs a mechanism for multiple clipboard sources.

Re: context, I currently have struct clipboard_state inited at startup and owned by MPContext, and it'd be easy enough to add an explicit uninit at shutdown. I don't think that needs to be added in this PR, though, since nothing in it precludes adding explicit shutdown function later.

Re: X/Wayland, I think we'd want to either have both run continuously (can they run in a single thread, select()ing both?) and return data from whichever was updated most recently, or have a user setting for which to use (defaulting to whichever we'd use for vo).

can they run in a single thread, select()ing both?

It's possible, but it will significantly complicate the implementation. This is especially true for large data transfers on X11, which manifest as a complicated multi-step process, for which the transfer state need to be saved and resumed between calls.

Personally I would like to limit clipboard to 1 backend at a time, to keep it sane. But dynamic switching would need to be implemented, like for VOs. That switching needs to take care of init/uninit on backend switches.

(Side note: not sure about the situation on macOS, but is XQuartz supported by mpv? That would mean that macOS has multiple clipboard backends too.)

Personally I would like to limit clipboard to 1 backend at a time, to keep it sane.

Agreed.

I think a reasonable way would be to delegate/marry this to a VO. Then the event loop can be reused to handle the necessary clipboard protocol.
Then add a way to have a global clipboard manager for the sake of win32 and mac (where this doesn't depend on a VO).

I think a reasonable way would be to delegate/marry this to a VO. Then the event loop can be reused to handle the necessary clipboard protocol.

Note that win32 also benefits from this: win32 clipboard notification requires a window, so without VO dependence, it is achieved by creating a dummy event-only window and wait for clipboard change messages. If it is delegated to the VO window instead, the window event loop can be reused.

This also provides a reasonable way out for Wayland compositors which don't support the wlr data control protocol.

Microsoft documentation explicitly discourages polling GetClipboardSequenceNumber():

Note that this is a not a notification method and should not be used in a polling loop. To be notified when clipboard contents change, use a clipboard format listener or a clipboard viewer.

Although this might not be a problem in practice if mpv does not call the function too frequently.

Also OpenClipboard() documentation says using NULL as window causes SetClipboardData to fail, although several unofficial sources report that it does not fail in practice on win32.

About X/Wayland, I think a vo-based API (using VOCTRLs) would be the best solution since it solves both the problem of persistent state and Wayland restriction. Although this may have some problems with frequent polling.

Ah, hmm, I'd missed those caveats; m_clipboard_poll is called on every render loop iteration, so that's probably not the best solution.

I think this could be worked around by letting the vo monitor clipboard through change events, and on clipboard changes, it runs a command to update a variable in player core, which m_clipboard_poll checks. This way m_clipboard_poll does not need to stop the vo thread.

Alternately, the vo could monitor clipboard events and fire mp_notify_property when applicable (might need a dispatch call?), and m_clipboard_poll could remain a no-op on windows. Not sure if any of this actually implies changes to the API as implemented here at all, then; the vo can just reach into mpctx->clipboard to hand it a HWND to pass to OpenClipboard when performing read/write calls.

The problem is that the current mpv architecture is not designed to make synchronous calls from somewhere down the call hierarchy (vo) to somewhere up (player core). Such usage is a recipe for deadlock: see tech-overview.txt, "Avoiding callback hell" section. Commands (through input_ctx) are safe to call from anywhere because they're asynchronous.

As mentioned before, the win32 concerns may be largely theoretical, so the API usage pattern can be the same as macOS. I see no large problem with the current API design for now if X/Wayland handle the clipboard in the vo thread, as there is no special init/uninit functions required.

I definitely see the usefulness but maybe copying images is better left out of the initial implementation to keep it simple first.

Screenshot support is pretty core to this functionality (it's one of, like, 2 main use-cases I'm targeting), so I'm hesitant to remove it unless we have a specific reason.

My thought was that it might be better to have a fleshed-out API for the "copy text/URL(s) to the clipboard" first, merge that and then integrate image clipboards into that (in a different PR). Perhaps even in a way that can be used from the client API.

I don't think we have any other properties that do this? What's the intended use case for this behavor?

Everything that uses m_property_read_sub does this (this doesn't use that to avoid reading every clipboard whenever a single one is requested, though maybe I could add a callback feature to m_sub_property to address that).

The manual doesn't mention this for any other properties so that might be what tripped me up.

So get_property_native("clipboard") will include text, url and paths yet get_property_string("clipboard/path") will work?
It doesn't seem wise to introduce such behaviour into properties.

I could include it; it's just redundant with paths, which would make it a little more annoying to use for development/debugging.

The "one global handler" approach will not work on Linux. Both the X11 and Wayland code needs to be compiled in, but only one of them will be active depending on the VO.

I'd expect to have a single linux implementation of the m_clipboard_* functions that dispatches between X and Wayland as needed.

Requiring only one implementation per platform sounds unnecessarily inflexible to me and as you proposed also leads to weird patterns where you have code that only forwards calls. Plus not being able to handle edge cases like X11 on macOS (just an example, not suggesting we should have that).

There can be multiple implementations of the underlying functionality per platform, they just need a implementation of the cross-platform APIs; if e.g. clipboard-linux.c wants to call into specific routines in clipboard-wayland.c and clipboard-x11.c, that's fine.

Personally I would like to limit clipboard to 1 backend at a time, to keep it sane.

Agreed.

I think a reasonable way would be to delegate/marry this to a VO. Then the event loop can be reused to handle the necessary clipboard protocol.
Then add a way to have a global clipboard manager for the sake of win32 and mac (where this doesn't depend on a VO).

Also could we use mp_ prefix like with all other things?

I'm not entirely clear on what the style guide situation is for when we use m_ vs mp_; I see a lot of usage of both within the codebase.

I would prefer to use mp_ like we use for everything recently.